Fix RequestResponseIO parseAndThrow to preserve retryable exception types by PDGGK · Pull Request #37342 · apache/beam

PDGGK · 2026-01-19T02:39:18Z

Summary

Fixes #37176 — RequestResponseIO loses retryable exception types in parseAndThrow.

Call.parseAndThrow unwrapped the ExecutionException from Future.get() but only special-cased
UserCodeQuotaException. Every other UserCodeExecutionException subclass — notably
UserCodeTimeoutException and UserCodeRemoteSystemException — was re-wrapped in a plain
UserCodeExecutionException, so Call's repeater saw shouldRepeat() == false and gave up on failures
that were meant to be retried.

Fix

Future.get() wraps a user exception in exactly one ExecutionException, so a single-level
e.getCause() check is sufficient:

Throwable cause = e.getCause();
if (cause instanceof UserCodeExecutionException) {
  throw (UserCodeExecutionException) cause;
}
throw new UserCodeExecutionException(cause == null ? e : cause);

Rethrowing the cause as-is preserves the concrete subclass rather than enumerating known ones, so it also
covers any UserCodeExecutionException subclass added later. The null-cause path still produces a
UserCodeExecutionException wrapping the original ExecutionException, as before.

An earlier revision of this PR walked the whole causal chain with Guava's Throwables.getCausalChain().
That was dropped — the deep-nesting cases it handled cannot occur on this code path — and the tests that
only exercised the traversal were removed with it.

Changes

sdks/java/io/rrio/.../Call.java — parseAndThrow rethrows the cause when it already is a
UserCodeExecutionException.
sdks/java/io/rrio/.../CallTest.java — two new tests plus one retargeted:
- givenCallerThrowsRemoteSystemException_thenPreservesExceptionType — asserts the failure carries
  UserCodeRemoteSystemException and not a bare UserCodeExecutionException.
- givenCallerThrowsNonUserCodeException_emitsWrappedUserCodeExecutionException — a plain
  RuntimeException is still wrapped, so the fallback path is covered.
- givenCallerThrowsTimeoutException_emitsFailurePCollection → renamed to
  ..._thenPreservesExceptionType and tightened: the bare-UserCodeExecutionException count now has to
  be 0, which is what actually fails without the fix.

Both *_thenPreservesExceptionType tests are regression tests: they assert the bare
UserCodeExecutionException count is 0, which only holds with the Call.java change.

gemini-code-assist · 2026-01-19T02:39:36Z

Summary of Changes

Hello @PDGGK, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical issue where retryable exceptions were being incorrectly wrapped by a generic UserCodeExecutionException, thereby breaking the retry mechanism. The core change involves enhancing the exception parsing logic to traverse the full causal chain, ensuring that specific retryable exception types are preserved and re-thrown. This significantly improves the reliability of retry logic within the RequestResponseIO component.

Highlights

Exception Preservation: The parseAndThrow method in Call.java has been refactored to correctly identify and re-throw specific retryable exceptions (UserCodeQuotaException, UserCodeTimeoutException, UserCodeRemoteSystemException) even when they are deeply nested within other exceptions, preventing them from being generically wrapped by UserCodeExecutionException.
Causal Chain Traversal: The fix utilizes Guava's Throwables.getCausalChain() to thoroughly inspect the entire exception causal chain, ensuring that the original retryable exception type is not lost due to intermediate wrapping.
Circular Reference Handling: The updated logic now gracefully handles circular references within an exception's causal chain, preventing infinite loops and throwing a UserCodeExecutionException with a descriptive message in such cases.
Comprehensive Testing: Ten new unit tests have been added to CallTest.java to cover various scenarios, including direct, nested, and triple-nested retryable exceptions, generic exceptions wrapping retryable ones, non-UserCode exceptions, and circular exception chains, ensuring the robustness of the fix.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

github-actions · 2026-01-19T03:39:11Z

Assigning reviewers:

R: @kennknowles for label java.

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

github-actions · 2026-01-26T12:19:17Z

Reminder, please take a look at this pr: @kennknowles

damccorm · 2026-01-26T15:28:19Z

@kennknowles could you please take a look?

github-actions · 2026-02-03T12:23:50Z

Reminder, please take a look at this pr: @kennknowles

github-actions · 2026-02-11T12:29:57Z

Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment assign to next reviewer:

R: @Abacn for label java.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

github-actions · 2026-02-19T12:26:20Z

Reminder, please take a look at this pr: @Abacn

kennknowles · 2026-02-20T16:14:39Z

I don't really remember the design well enough to quickly review just the diff. I would need to read the whole IO to understand the intent. Maybe @Abacn knows better than me. I have a couple questions / comments

Who is doing the retrying? Why are they not retrying the UserCodeException? Should they be? if it is inside the RRIO then how did it get wrapped into UserCodeException? If it is the runner, then UserCodeException is usually the correct way to pass the info to the runner. I just don't know the design here.
It seems very odd (like, something I've never seen in any code) to traverse the whole causal chain. I feel there must be a more straightforward thing we should do.

PDGGK · 2026-02-22T07:39:50Z

Thanks for the questions @kennknowles!

Who is doing the retrying?
Retrying happens inside RRIO, in Repeater.apply() — not by the runner. Repeater catches UserCodeExecutionException and calls e.shouldRepeat() to decide whether to retry. The three subclasses (UserCodeQuotaException, UserCodeTimeoutException, UserCodeRemoteSystemException) override shouldRepeat() to return true, while the base class returns false.

The bug is that parseAndThrow only preserved UserCodeQuotaException when unwrapping ExecutionException from Future.get(). The other two retryable types were being re-wrapped as base UserCodeExecutionException, which made shouldRepeat() return false — silently skipping retries for timeout and remote system errors.

Traversing the full causal chain
You're right — that's unnecessarily complex. Future.get() adds exactly one ExecutionException wrapper, so inspecting e.getCause() is sufficient. I'll simplify parseAndThrow to: if the cause is already a UserCodeExecutionException (or subclass), rethrow it directly; otherwise wrap it once. This is simpler and automatically handles any future subclasses.

Pushing the simplified version now.

github-actions · 2026-03-01T12:19:10Z

Reminder, please take a look at this pr: @Abacn @kennknowles

github-actions · 2026-03-04T12:23:54Z

Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment assign to next reviewer:

R: @ahmedabu98 for label java.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

github-actions · 2026-03-11T12:26:11Z

Reminder, please take a look at this pr: @ahmedabu98 @kennknowles

github-actions · 2026-03-16T12:32:09Z

Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment assign to next reviewer:

R: @chamikaramj for label java.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

github-actions · 2026-05-18T13:33:07Z

Reminder, please take a look at this pr: @ahmedabu98 @kennknowles

github-actions · 2026-05-21T13:25:00Z

Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment assign to next reviewer:

R: @chamikaramj for label java.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

github-actions · 2026-05-28T13:33:28Z

Reminder, please take a look at this pr: @chamikaramj @kennknowles

github-actions · 2026-06-01T14:20:29Z

Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment assign to next reviewer:

R: @ahmedabu98 for label java.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

github-actions · 2026-06-09T13:15:59Z

Reminder, please take a look at this pr: @ahmedabu98 @kennknowles

github-actions · 2026-06-11T13:40:27Z

Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment assign to next reviewer:

R: @Abacn for label java.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

damccorm · 2026-06-12T17:35:00Z

@kennknowles what are next steps here?

github-actions · 2026-06-20T12:51:20Z

Reminder, please take a look at this pr: @Abacn @kennknowles

github-actions · 2026-06-24T13:02:49Z

Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment assign to next reviewer:

R: @chamikaramj for label java.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

PDGGK · 2026-06-29T07:58:39Z

Friendly ping — this one's been waiting on review since February and CI is green / the branch is mergeable.

The questions @kennknowles raised earlier are resolved: per the feedback I simplified parseAndThrow to inspect e.getCause() once (Future.get() adds exactly one ExecutionException wrapper) — if the cause is a UserCodeExecutionException or subclass it's rethrown as-is so shouldRepeat() still sees the retryable type, otherwise it's wrapped once. That's in commit 5ff10ae and the tests were updated to match.

@chamikaramj @Abacn @kennknowles could one of you take a look when you have a chance? Happy to make any further changes.

github-actions · 2026-07-06T13:27:42Z

Reminder, please take a look at this pr: @chamikaramj @kennknowles

github-actions · 2026-07-09T13:24:37Z

Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment assign to next reviewer:

R: @Abacn for label java.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

bvolpato

LGTM. I traced the full path from CallerWithTimeout through Future.get() / parseAndThrow into Repeater. Rethrowing an existing UserCodeExecutionException is correct here: Future.get() adds one ExecutionException wrapper, and preserving the subtype keeps shouldRepeat() intact for timeout, quota, remote-system, and future subclasses. Non-RRIO exceptions remain wrapped once.

I also tested a clean synthetic merge with current master (RRIO files changed since this PR's last CI run): full :sdks:java:io:rrio:test, Spotless, and Checkstyle on Java 17, plus CallTest and RequestResponseIOTest on Java 21. All green.

Non-blocking: PR description still describes causal-chain traversal and 10 new tests, but current patch uses one-level unwrapping and a smaller test set. Worth updating before merge.

github-actions · 2026-07-21T12:42:34Z

Reminder, please take a look at this pr: @Abacn @kennknowles

github-actions · 2026-07-23T12:43:28Z

Assigning new set of reviewers because Pr has gone too long without review. If you would like to opt out of this review, comment assign to next reviewer:

R: @ahmedabu98 for label java.

Available commands:

stop reviewer notifications - opt out of the automated review tooling
remind me after tests pass - tag the comment author after tests pass
waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

…ypes Problem: The parseAndThrow method in Call.java was wrapping retryable exceptions (UserCodeTimeoutException, UserCodeRemoteSystemException) in a generic UserCodeExecutionException, which breaks the retry logic that depends on exception.shouldRepeat() returning true. Solution: - Scan the full causal chain using Guava's Throwables.getCausalChain() - Preserve all specific retryable exception types (Quota/Timeout/RemoteSystem) - Prefer specific types over generic UserCodeExecutionException when both exist in the chain to prevent masking of retryable exceptions - Handle circular causal chains gracefully by catching IllegalArgumentException Testing: - Added 10 new unit tests covering: * Direct retryable exceptions (Timeout, RemoteSystem) * Nested exceptions (UncheckedExecutionException wrapping) * Generic UserCodeExecutionException masking specific types * Triple-nested exceptions * Circular reference in causal chain * Non-UserCode exceptions (RuntimeException) - All existing tests pass - Full rrio test suite passes (90 tasks) Fixes apache#37176

…al chain Future.get() wraps with exactly one ExecutionException, so inspecting e.getCause() is sufficient. If the cause is a UserCodeExecutionException (or subclass), rethrow it directly - this preserves all retryable types and is automatically future-proof for new subclasses. Remove deep-nesting tests that tested the now-removed causal chain traversal, as those scenarios cannot occur in this code path.

github-actions Bot added java io labels Jan 19, 2026

PDGGK mentioned this pull request Jan 19, 2026

Fix RequestResponseIO parseAndThrow to preserve retryable exception types #37341

Closed

9 tasks

github-actions Bot added the Next Action: Reviewers label Jan 19, 2026

PDGGK mentioned this pull request Jan 23, 2026

[Bug]: RequestResponseIO: Call wraps retryable exceptions in UserCodeExecutionException, preventing retry/backoff #37176

Open

github-actions Bot added the slow-review label Jan 26, 2026

github-actions Bot removed the slow-review label Jan 26, 2026

github-actions Bot added the slow-review label Feb 3, 2026

kennknowles self-requested a review February 4, 2026 17:20

github-actions Bot added reassigned-reviewers and removed slow-review labels Feb 11, 2026

github-actions Bot added the slow-review label Feb 19, 2026

github-actions Bot removed the slow-review label Feb 20, 2026

github-actions Bot added the slow-review label Mar 1, 2026

github-actions Bot removed the slow-review label Mar 4, 2026

github-actions Bot added the slow-review label Mar 11, 2026

github-actions Bot removed the slow-review label Mar 16, 2026

github-actions Bot added the slow-review label May 18, 2026

github-actions Bot removed the slow-review label May 21, 2026

github-actions Bot added the slow-review label May 28, 2026

github-actions Bot removed the slow-review label Jun 1, 2026

github-actions Bot added the slow-review label Jun 9, 2026

github-actions Bot removed the slow-review label Jun 11, 2026

github-actions Bot added the slow-review label Jun 20, 2026

github-actions Bot removed the slow-review label Jun 24, 2026

github-actions Bot added the slow-review label Jul 6, 2026

github-actions Bot removed the slow-review label Jul 9, 2026

bvolpato approved these changes Jul 13, 2026

View reviewed changes

github-actions Bot added the slow-review label Jul 21, 2026

github-actions Bot removed the slow-review label Jul 23, 2026

PDGGK added 2 commits July 26, 2026 02:36

PDGGK force-pushed the java-fix-requestresponseio branch from 5ff10ae to 819af20 Compare July 25, 2026 16:36

Uh oh!

Conversation

PDGGK commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Fix

Changes

Uh oh!

gemini-code-assist Bot commented Jan 19, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

github-actions Bot commented Jan 19, 2026

Uh oh!

github-actions Bot commented Jan 26, 2026

Uh oh!

damccorm commented Jan 26, 2026

Uh oh!

github-actions Bot commented Feb 3, 2026

Uh oh!

github-actions Bot commented Feb 11, 2026

Uh oh!

github-actions Bot commented Feb 19, 2026

Uh oh!

kennknowles commented Feb 20, 2026

Uh oh!

PDGGK commented Feb 22, 2026

Uh oh!

github-actions Bot commented Mar 1, 2026

Uh oh!

github-actions Bot commented Mar 4, 2026

Uh oh!

github-actions Bot commented Mar 11, 2026

Uh oh!

github-actions Bot commented Mar 16, 2026

Uh oh!

github-actions Bot commented May 18, 2026

Uh oh!

github-actions Bot commented May 21, 2026

Uh oh!

github-actions Bot commented May 28, 2026

Uh oh!

github-actions Bot commented Jun 1, 2026

Uh oh!

github-actions Bot commented Jun 9, 2026

Uh oh!

github-actions Bot commented Jun 11, 2026

Uh oh!

damccorm commented Jun 12, 2026

Uh oh!

github-actions Bot commented Jun 20, 2026

Uh oh!

github-actions Bot commented Jun 24, 2026

Uh oh!

PDGGK commented Jun 29, 2026

Uh oh!

github-actions Bot commented Jul 6, 2026

Uh oh!

github-actions Bot commented Jul 9, 2026

Uh oh!

bvolpato left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Jul 21, 2026

Uh oh!

github-actions Bot commented Jul 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

PDGGK commented Jan 19, 2026 •

edited

Loading